From f85414f956304414064de77e15b8c50b13d01958 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 14 Sep 2016 18:37:35 -0700 Subject: [PATCH] Avoid MWException and wf* log methods in DatabaseBase Change-Id: Idc418ae1088f87d6416e2552976d94f7d1e8f5db --- includes/db/Database.php | 55 ++++++++++--------- includes/db/DatabaseMssql.php | 5 +- includes/db/DatabaseSqlite.php | 2 +- includes/db/loadbalancer/LBFactoryMulti.php | 4 +- includes/db/loadbalancer/LBFactorySimple.php | 2 +- .../libs/rdbms/loadbalancer/LoadBalancer.php | 2 +- 6 files changed, 35 insertions(+), 35 deletions(-) diff --git a/includes/db/Database.php b/includes/db/Database.php index e68a8f25f2..0c357cc9ed 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -69,6 +69,8 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { protected $connLogger; /** @var LoggerInterface */ protected $queryLogger; + /** @var callback Error logging callback */ + protected $errorLogger; /** @var resource Database connection */ protected $mConn = null; @@ -319,7 +321,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { * @param array $p An array of options to pass to the constructor. * Valid options are: host, user, password, dbname, flags, tablePrefix, schema, driver * @return IDatabase|null If the database driver or extension cannot be found - * @throws MWException + * @throws InvalidArgumentException If the database driver or extension cannot be found */ final public static function factory( $dbType, $p = [] ) { global $wgCommandLineMode; @@ -355,7 +357,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { $driver = $dbType; } if ( $driver === false ) { - throw new MWException( __METHOD__ . + throw new InvalidArgumentException( __METHOD__ . " no viable database extension found for type '$dbType'" ); } @@ -390,6 +392,11 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { if ( isset( $p['queryLogger'] ) ) { $conn->queryLogger = $p['queryLogger']; } + if ( isset( $p['errorLogger'] ) ) { + $conn->errorLogger = $p['errorLogger']; + } else { + $conn->errorLogger = [ MWExceptionHandler::class, 'logException' ]; + } } else { $conn = null; } @@ -741,7 +748,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { protected function installErrorHandler() { $this->mPHPError = false; $this->htmlErrors = ini_set( 'html_errors', '0' ); - set_error_handler( [ $this, 'connectionErrorHandler' ] ); + set_error_handler( [ $this, 'connectionerrorLogger' ] ); } /** @@ -766,12 +773,12 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { * @param int $errno * @param string $errstr */ - public function connectionErrorHandler( $errno, $errstr ) { + public function connectionerrorLogger( $errno, $errstr ) { $this->mPHPError = $errstr; } /** - * Create a log context to pass to wfLogDBError or other logging functions. + * Create a log context to pass to PSR logging functions. * * @param array $extras Additional data to add to context * @return array @@ -790,11 +797,6 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { public function close() { if ( $this->mConn ) { if ( $this->trxLevel() ) { - if ( !$this->mTrxAutomatic ) { - wfWarn( "Transaction still in progress (from {$this->mTrxFname}), " . - " performing implicit commit before closing connection!" ); - } - $this->commit( __METHOD__, self::FLUSHING_INTERNAL ); } @@ -948,7 +950,8 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { if ( $this->reconnect() ) { $msg = __METHOD__ . ": lost connection to {$this->getServer()}; reconnected"; $this->connLogger->warning( $msg ); - $this->queryLogger->warning( "$msg:\n" . wfBacktrace( true ) ); + $this->queryLogger->warning( + "$msg:\n" . ( new RuntimeException() )->getTraceAsString() ); if ( !$recoverable ) { # Callers may catch the exception and continue to use the DB @@ -1097,7 +1100,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { $this->queryLogger->debug( "SQL ERROR (ignored): $error\n" ); } else { $sql1line = mb_substr( str_replace( "\n", "\\n", $sql ), 0, 5 * 1024 ); - wfLogDBError( + $this->queryLogger->error( "{fname}\t{db_server}\t{errno}\t{error}\t{sql1line}", $this->getLogContext( [ 'method' => __METHOD__, @@ -2805,7 +2808,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { $this->clearFlag( DBO_TRX ); // restore auto-commit } } catch ( Exception $ex ) { - MWExceptionHandler::logException( $ex ); + call_user_func( $this->errorLogger, $ex ); $e = $e ?: $ex; // Some callbacks may use startAtomic/endAtomic, so make sure // their transactions are ended so other callbacks don't fail @@ -2839,7 +2842,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { list( $phpCallback ) = $callback; call_user_func( $phpCallback ); } catch ( Exception $ex ) { - MWExceptionHandler::logException( $ex ); + call_user_func( $this->errorLogger, $ex ); $e = $e ?: $ex; } } @@ -2872,7 +2875,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { list( $phpCallback ) = $callback; $phpCallback( $trigger, $this ); } catch ( Exception $ex ) { - MWExceptionHandler::logException( $ex ); + call_user_func( $this->errorLogger, $ex ); $e = $e ?: $ex; } } @@ -2936,15 +2939,13 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { } else { // @TODO: make this an exception at some point $msg = "$fname: Implicit transaction already active (from {$this->mTrxFname})."; - wfLogDBError( $msg ); - wfWarn( $msg ); + $this->queryLogger->error( $msg ); return; // join the main transaction set } } elseif ( $this->getFlag( DBO_TRX ) && $mode !== self::TRANSACTION_INTERNAL ) { // @TODO: make this an exception at some point $msg = "$fname: Implicit transaction expected (DBO_TRX set)."; - wfLogDBError( $msg ); - wfWarn( $msg ); + $this->queryLogger->error( $msg ); return; // let any writes be in the main transaction } @@ -3003,13 +3004,12 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { } } else { if ( !$this->mTrxLevel ) { - wfWarn( "$fname: No transaction to commit, something got out of sync." ); + $this->queryLogger->error( "$fname: No transaction to commit, something got out of sync." ); return; // nothing to do } elseif ( $this->mTrxAutomatic ) { // @TODO: make this an exception at some point $msg = "$fname: Explicit commit of implicit transaction."; - wfLogDBError( $msg ); - wfWarn( $msg ); + $this->queryLogger->error( $msg ); return; // wait for the main transaction set commit round } } @@ -3050,7 +3050,8 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { } } else { if ( !$this->mTrxLevel ) { - wfWarn( "$fname: No transaction to rollback, something got out of sync." ); + $this->queryLogger->error( + "$fname: No transaction to rollback, something got out of sync." ); return; // nothing to do } elseif ( $this->getFlag( DBO_TRX ) ) { throw new DBUnexpectedError( @@ -3119,7 +3120,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { * @param string $newName Name of table to be created * @param bool $temporary Whether the new table should be temporary * @param string $fname Calling function name - * @throws MWException + * @throws RuntimeException * @return bool True if operation was successful */ public function duplicateTableStructure( $oldName, $newName, $temporary = false, @@ -3148,7 +3149,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { * * @param string $prefix Only show VIEWs with this prefix, eg. unit_test_ * @param string $fname Name of calling function - * @throws MWException + * @throws RuntimeException * @return array * @since 1.22 */ @@ -3160,7 +3161,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { * Differentiates between a TABLE and a VIEW * * @param string $name Name of the database-structure to test. - * @throws MWException + * @throws RuntimeException * @return bool * @since 1.22 */ @@ -3349,8 +3350,8 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface { * generated dynamically using $filename * @param bool|callable $inputCallback Optional function called for each * complete line sent - * @throws Exception|MWException * @return bool|string + * @throws Exception */ public function sourceFile( $filename, $lineCallback = false, $resultCallback = false, $fname = false, $inputCallback = false diff --git a/includes/db/DatabaseMssql.php b/includes/db/DatabaseMssql.php index 2439c60073..f39f49121d 100644 --- a/includes/db/DatabaseMssql.php +++ b/includes/db/DatabaseMssql.php @@ -777,7 +777,6 @@ class DatabaseMssql extends Database { * @return bool * @throws DBUnexpectedError * @throws Exception - * @throws MWException */ function update( $table, $values, $conds, $fname = __METHOD__, $options = [] ) { $table = $this->tableName( $table ); @@ -814,7 +813,7 @@ class DatabaseMssql extends Database { * @param array $binaryColumns Contains a list of column names that are binary types * This is a custom parameter only present for MS SQL. * - * @throws MWException|DBUnexpectedError + * @throws DBUnexpectedError * @return string */ public function makeList( $a, $mode = LIST_COMMA, $binaryColumns = [] ) { @@ -1074,7 +1073,7 @@ class DatabaseMssql extends Database { * Throws an exception if it is invalid. * Reference: http://msdn.microsoft.com/en-us/library/aa224033%28v=SQL.80%29.aspx * @param string $identifier - * @throws MWException + * @throws InvalidArgumentException * @return string */ private function escapeIdentifier( $identifier ) { diff --git a/includes/db/DatabaseSqlite.php b/includes/db/DatabaseSqlite.php index 6bf48e2365..6663ec51d5 100644 --- a/includes/db/DatabaseSqlite.php +++ b/includes/db/DatabaseSqlite.php @@ -944,12 +944,12 @@ class DatabaseSqlite extends Database { } /** - * @throws MWException * @param string $oldName * @param string $newName * @param bool $temporary * @param string $fname * @return bool|ResultWrapper + * @throws RuntimeException */ function duplicateTableStructure( $oldName, $newName, $temporary = false, $fname = __METHOD__ ) { $res = $this->query( "SELECT sql FROM sqlite_master WHERE tbl_name=" . diff --git a/includes/db/loadbalancer/LBFactoryMulti.php b/includes/db/loadbalancer/LBFactoryMulti.php index dd7737bb1c..bbee3b8783 100644 --- a/includes/db/loadbalancer/LBFactoryMulti.php +++ b/includes/db/loadbalancer/LBFactoryMulti.php @@ -164,7 +164,7 @@ class LBFactoryMulti extends LBFactory { /** * @param array $conf - * @throws MWException + * @throws InvalidArgumentException */ public function __construct( array $conf ) { parent::__construct( $conf ); @@ -264,7 +264,7 @@ class LBFactoryMulti extends LBFactory { /** * @param string $cluster * @param bool|string $wiki - * @throws MWException + * @throws InvalidArgumentException * @return LoadBalancer */ protected function newExternalLB( $cluster, $wiki = false ) { diff --git a/includes/db/loadbalancer/LBFactorySimple.php b/includes/db/loadbalancer/LBFactorySimple.php index d8590b7003..908453caa5 100644 --- a/includes/db/loadbalancer/LBFactorySimple.php +++ b/includes/db/loadbalancer/LBFactorySimple.php @@ -102,10 +102,10 @@ class LBFactorySimple extends LBFactory { } /** - * @throws MWException * @param string $cluster * @param bool|string $wiki * @return LoadBalancer + * @throws InvalidArgumentException */ protected function newExternalLB( $cluster, $wiki = false ) { global $wgExternalServers; diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index 6f136df75e..903c160968 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -776,7 +776,7 @@ class LoadBalancer implements ILoadBalancer { * @param bool $dbNameOverride * @return IDatabase * @throws DBAccessError - * @throws MWException + * @throws InvalidArgumentException */ protected function reallyOpenConnection( $server, $dbNameOverride = false ) { if ( $this->disabled ) { -- 2.20.1